-
-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor channel interfaces #1115
Conversation
Make MemorySendChannel and MemoryReceiveChannel into public classes, and move a number of the methods that used to be on the abstract SendChannel/ReceiveChannel interfaces into the Memory*Channel concrete classes. Also add a Channel type, analogous to Stream. See: python-triogh-719 Still to do: - Merge MemorySendChannel and MemoryReceiveChannel into a single MemoryChannel - decide what to do about clone - decide whether to add some kind of half-close on Channel - refactor this PR's one-off solution to python-triogh-1092 into something more general.
Codecov Report
@@ Coverage Diff @@
## master #1115 +/- ##
==========================================
+ Coverage 99.51% 99.51% +<.01%
==========================================
Files 102 102
Lines 12518 12526 +8
Branches 953 953
==========================================
+ Hits 12457 12465 +8
Misses 40 40
Partials 21 21
|
Codecov Report
@@ Coverage Diff @@
## master #1115 +/- ##
==========================================
+ Coverage 99.51% 99.51% +<.01%
==========================================
Files 102 102
Lines 12518 12529 +11
Branches 953 950 -3
==========================================
+ Hits 12457 12468 +11
Misses 40 40
Partials 21 21
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a super valuable simplification overall, thanks for making it! A couple doc comments.
"""Attempt to receive an incoming object, blocking if necessary. | ||
|
||
It's legal for multiple tasks to call :meth:`receive` at the same |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like it would be useful to explicitly document what is and is not guaranteed about tasks calling send() or receive() simultaneously for all Channels. Also maybe something about cancellation safety. If you want to stop guaranteeing that multiple simultaneous receive() works OK, maybe move this text to MemoryReceiveChannel.receive() instead of removing it entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah. I'm thinking ahead to "framing channels" that just wrap line-breaking or whatever around a stream, so they'll have the same single-user rule as streams. But that's a good point.
Make MemorySendChannel and MemoryReceiveChannel into public classes,
and move a number of the methods that used to be on the abstract
SendChannel/ReceiveChannel interfaces into the Memory*Channel concrete
classes. Also add a Channel type, analogous to Stream.
See: gh-719
Still to do:
MemoryChannel
refactor this PR's one-off solution to Should the Final and NoPublicConstructor classes inherit from typing.GenericMeta? #1092 into something moredonegeneral.